fix(provisioning): Do not provision mail account for users without access to mail app #12226
Conversation
DerDreschner
requested review from
ChristophWurst,
GretaD and
kesselb
as code owners
DerDreschner
force-pushed
the
fix/check-for-disabled-app
branch
from
27e6211 to
6b43c71
Compare
|
@ChristophWurst : Should I add a migration to remove mail accounts from users that have no access to the mail app...? Or is that already being handled by a background job, etc.? |
|
There's https://github.com/nextcloud/mail/blob/main/lib/Migration/ProvisionAccounts.php to provision new accounts. The case that a user had once access to the mail app but not anymore is currently not implemented. We should call |
|
@kesselb : Great, I'll add that here later! Thanks! |
Signed-off-by: David Dreschner <[email protected]>
DerDreschner
force-pushed
the
fix/check-for-disabled-app
branch
from
6b43c71 to
c69f573
Compare
|
I've implemented the unprovisioning and changed the test. I'm a bit unsure if there would be a better way for the unit test? Because it doesn't test if there is an actual unprovisioning happen as the database command just return |
| /** | ||
| * Delete all provisioned aliases for the given uid | ||
| * | ||
| * Exception for Nextcloud 20: \Doctrine\DBAL\DBALException |
There was a problem hiding this comment.
As we're far away from NC 20/21, I think it's okay to change that here now?
3 participants
When using the provisioning functionality, only the configured pattern where checked before pushing the mail account configuration into the database. This is fine as long as all users can use the mail app. But as the access to specific apps can be limited to specific groups, this can lead to accounts that are configured and synced in the background, yet being never used and only eat up precious resources.
To ensure this does not happen anymore, I use the
IAppManagerto check if the specific user has the right to use the mail app. If not, the provisioning for the user will be stopped.This fixes #12057.